Added [System.Flags] to enums that are flags. - #354
Conversation
…just in case they come up in the future.
There was a problem hiding this comment.
Pull request overview
This PR updates the unsafe generator and regenerated enum outputs so that enums whose members are all expressed as single-bit shifts (1 << n) are emitted as C# flags enums ([Flags]) and preserve the shift-form instead of collapsing to numeric literals.
Changes:
- Updated enum processing to preserve
= 1 << nexpressions in generated enum member values (with appropriate numeric literal suffixes). - Updated enum generation to emit
using System;and add[Flags]to enums where every member value contains a left-shift. - Regenerated
Enums.g.csoutputs to reflect[Flags]and1 << nformatting for the affected enums (e.g.,SwsFlags,AVTimecodeFlag).
Reviewed changes
Copilot reviewed 2 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| FFmpeg.AutoGen/generated/Enums.g.cs | Regenerated enums now include using System;, [Flags] on identified flags enums, and 1 << n member values. |
| FFmpeg.AutoGen.CppSharpUnsafeGenerator/Processing/EnumerationProcessor.cs | Preserves = 1 << n expressions during enum item value conversion via a generated regex. |
| FFmpeg.AutoGen.CppSharpUnsafeGenerator/Generation/EnumsGenerator.cs | Emits using System; and adds [Flags] when all enum items contain <<. |
| FFmpeg.AutoGen.Abstractions/generated/Enums.g.cs | Same regeneration as main package enums: using System;, [Flags], and 1 << n values where applicable. |
# Conflicts: # FFmpeg.AutoGen.Abstractions/generated/Enums.g.cs # FFmpeg.AutoGen/generated/Enums.g.cs
The detection keyed on the header writing a member as "1 << n", which makes the result depend on FFmpeg's source formatting rather than on what the enum means. SWS_BACKEND_C is written "(1 << 1)" and the regex could not match the closing parenthesis; AV_CODEC_HW_CONFIG_METHOD_* and AV_SIDE_DATA_PARAM_CHANGE_* are written in hex. All are bitmasks by their own header documentation and all were missed, and a reformatting upstream could turn the attribute on or off with no change here. EnumerationDefinition now carries IsFlags, computed in the processor from the numeric values: at least two distinct single-bit members, at most a couple of named aggregates such as SWS_BACKEND_ALL, every aggregate covered by bits the enum introduces, sentinels like *_MAX_ENUM ignored, and a consecutive run rejected so that 0, 1, 2 stays a sequence. EnumsGenerator reads that flag instead of sniffing for "<<" in the emitted text. Marks four more: AvCodecHwConfigMethod, AVSideDataParamChangeFlags, AVSideDataProps and SwsBackend. Drops one: AvFrameCrop has a single member, and values alone cannot tell AV_FRAME_CROP_UNALIGNED from AV_HWFRAME_TRANSFER_DIRECTION_FROM/TO, which have the same shape and are a direction rather than a mask. Keeping the shift spelling is now a separate concern from detection, and it is verified: the literal is only kept when 1 << n actually reproduces the value the compiler computed, since C# masks shift counts and would otherwise emit a different constant. The suffix comes from the type name the enum is generated with rather than from a second table of primitives, which had no cases for LongLong and ULongLong - the two that really are C# long and ulong - and disagreed with TypeHelper about Long and ULong. EnumerationItem keeps a numeric Value and gains Literal. Value is what ASTProcessor publishes into WellKnownEnumItems, where it is spliced into macro bodies as text, so an unparenthesised "1 << 3" there would have changed the precedence of any macro referring to the member. Also: invariant culture when formatting values, since several locales render the negative sign as U+2212 and the generated file is C# source; a named regex group per CONTRIBUTING; and "using System;" only when something is actually marked. Co-authored-by: Daniel Sass <twindan@users.noreply.github.com>
|
Thanks for this, and sorry it sat for so long. Two things I want to say before the changes: you filed the issue, implemented it, and then came back and rebased onto 9.0.1.1 the same day it shipped. And the generated files in the branch reproduce byte-for-byte from your generator — I checked with I have pushed a commit onto your branch rather than sending you round another lap. Look it over and push back on anything you disagree with. What I changed and whyThe detection keyed on the header spelling a member as SWS_BACKEND_C = (1 << 1), // parenthesised - the regex is anchored to $
AV_CODEC_HW_CONFIG_METHOD_HW_DEVICE_CTX = 0x01, // hex
AV_SIDE_DATA_PARAM_CHANGE_SAMPLE_RATE = 0x0004, // hex, and the header's own docs use it as a maskAll three are documented bitmasks. The sharper worry is that this can flip with no change on our side: if upstream ever reformats So Net effect, 5 enums marked before, 8 now:
Keeping the shift spellingI kept it — reading like the header is genuinely useful when you are porting C code across — but it is now independent of detection and verified, because C# masks shift counts. The suffix comes from the type name the enum is generated with, rather than a second Smaller things
One question for you
Worth knowing for scale, by the way: FFmpeg keeps almost all of its flags in macros, not enums — 149 |
Description
Some enums that are conceptually flags (e.g., SwsFlags) do no have [System.Flags] on the enum definition. This code adds that definition for the relevant classes.
What I do is look at the enum definition in ffmpeg when parsing. If it is of the form "1 << n", then I preserve the definition instead of collapsing the value (so the generated file say "VALUE = 1 << 3" instead of "VALUE = 8"). If all items in an enum are of the form 1 << n, then I treat it as a [System.Flags]. This seems to be how ffmpeg uses enumerated values.
Type of Change
Related Issues
#353
Testing
Ran the output
dotnet build -c Releasesuccessfullydotnet test -c ReleasesuccessfullyChecklist
Additional Notes
Output is the same except for enums that now have [System.Flags] appended. I have included the new generated files in this PR. Ran against ffmpeg commit 9047fa1b.